Supports Anymal-D Rough terrain training in newton#5225
Supports Anymal-D Rough terrain training in newton#5225ooctipus wants to merge 1 commit intoisaac-sim:developfrom
Conversation
kellyguo11
left a comment
There was a problem hiding this comment.
PR Review: Supports Anymal-D Rough Terrain Training in Newton
Overview
This is a large, multi-feature PR (+5344/-3744 across 87 files) that bundles together several cross-cutting changes needed for Newton-based rough terrain locomotion training. The scope spans:
- Newton XformPrimView (
NewtonSiteXformPrimView) — GPU-resident site-based transform tracking via Warp kernels - Fabric XformPrimView (
FabricXformPrimView) — Fabric-accelerated variant for PhysX backend - XformPrimView contract tests (
xform_contract_tests.py) — Shared test invariants for all backends - Newton collision pipeline config (
NewtonCollisionPipelineCfg,HydroelasticSDFCfg) — Expose Newton collision parameters - Friction/restitution randomization —
set_friction_index/mask,set_restitution_index/maskfor Newton assets +set_material_properties_index/maskfor PhysX - Material randomization refactor — Backend-aware
randomize_rigid_body_material+ newrandomize_rigid_body_inertia+ refactoredrandomize_rigid_body_collider_offsets - RayCaster sensor spawning —
RayCasterXformCfgso sensors get their own non-physics Xform (Newton requires this) - New terminations —
body_lin_vel_out_of_limit,body_ang_vel_out_of_limit - Task config updates — All locomotion rough-terrain configs updated for raycaster child paths + Newton solver configs for AnymalD
- Velocity env restructure —
StartupEventsCfgmerged intoEventsCfg, events no longerMISSING
Should-Fix Issues
1. Version mismatch in isaaclab_newton extension.toml vs CHANGELOG
extension.toml sets version = "0.5.11", but the CHANGELOG has entries for 0.5.11, 0.5.12, and 0.5.13. The version in extension.toml should match the latest changelog entry (0.5.13).
2. Changelog entry for isaaclab_physx version ordering inconsistency
The CHANGELOG adds entries for 0.5.15 and 0.5.14, but extension.toml is bumped from 0.5.13 → 0.5.14. The 0.5.15 changelog entry documents work in this PR but there's no corresponding version bump, suggesting either the entry should be 0.5.14 or the toml needs to go to 0.5.15.
3. velocity_env_cfg.py: events field changed from MISSING to EventsCfg()
LocomotionVelocityRoughEnvCfg.events changes from MISSING to EventsCfg(). This is a breaking change — all downstream configs that previously had to supply events (like AnymalB, AnymalC, Spot, etc. not shown in this PR) will now silently get the base defaults. Configs that override events with their own dataclass (e.g., the old AnymalDPhysxEventsCfg(EventsCfg, StartupEventsCfg) pattern) are no longer needed since startup events moved into EventsCfg, but any external config relying on MISSING to enforce explicit event configuration will break silently.
Additionally, StartupEventsCfg is removed. Code outside the repo that inherits from it will fail at import.
4. randomize_rigid_body_material: Newton path uses set_friction_mask / set_restitution_mask (full data every call)
In _call_newton, the code always passes the full self.default_friction / self.default_restitution tensors via set_*_mask(), even when only a subset of environments changed. This writes the entire (num_instances × num_shapes) buffer every call. Consider using set_friction_index / set_restitution_index with only the env_ids that need updating, similar to how the PhysX path passes env_ids to set_material_properties_index.
5. NewtonSiteXformPrimView._gather_scales kernel has O(N×S) complexity
The _gather_scales Warp kernel does a linear scan over all shapes for every site (O(sites × shapes)). For articulations with hundreds of shapes, this is slow. Consider building a precomputed site_body → first_shape index at init time.
Suggestions
6. PR scope — consider splitting
This PR bundles at least 5 independent features. Each would be easier to review and less risky to merge independently:
- XformPrimView backends + contract tests
- Collision pipeline config
- Friction/restitution APIs + material randomization refactor
- RayCaster spawning refactor
- AnymalD Newton config + velocity env restructure
7. randomize_rigid_body_collider_offsets — Newton path accesses internal attributes
The Newton implementation reads/writes shape_margin and shape_gap via root_view.get_attribute() / root_view.set_attribute() with SimulationManager.get_model(). This couples the randomization term to Newton internals. A TODO comment acknowledging this would help.
8. base_com preset uses newton=None in velocity_env_cfg
The PR disables CoM randomization for Newton (base_com = preset(default=EventTerm(...), newton=None)). The PR description mentions "we still have to disable randomize_rigid_body_com, as turning it on will cause levitating robot". This should be documented as a known limitation with a TODO or link to a tracking issue.
9. Test comparison with Isaac Sim — reduced coverage
test_compare_get_world_poses_with_isaacsim and the remaining Isaac Sim comparison tests were heavily simplified — quaternion comparisons removed, local pose and set-pose comparisons deleted entirely. The contract tests partially compensate, but backend-specific behavioral parity with Isaac Sim is no longer tested.
10. UsdXformPrimView.set_world_poses — partial-update edge case
When only positions is provided (without orientations), the code reads the current world transform, replaces translation, then decomposes back to local. This round-trip through Gf.Matrix4d may accumulate floating-point drift over many calls. Consider documenting this or caching.
11. Documentation checkbox unchecked
The PR checklist shows [ ] I have made corresponding changes to the documentation. The raycaster tutorial docs are updated, but the new APIs (set_friction_index, NewtonCollisionPipelineCfg, RayCasterXformCfg, new terminations, randomize_rigid_body_inertia) should have corresponding API docs or migration guide entries.
Positive Notes
- The contract test pattern (
xform_contract_tests.pywithViewBundlefixture) is excellent engineering — ensures all backends satisfy the same invariants - The
NewtonSiteXformPrimViewdesign is clean: ancestor-walk resolution at init, GPU-only pose compute at runtime - The raycaster spawning refactor (
RayCasterXformCfg) is a good pattern — decoupling sensor attachment from physics prims - Backend-aware material randomization with clean PhysX/Newton dispatch
- Good deprecation handling with auto-redirect for legacy raycaster paths
f9bfc0e to
1a37500
Compare
Description
This PR depends on
#5219
#5179
#5098
Note:
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there